Make unused-import check all ancestors for typing guards
#5316
Conversation
DanielNoord
added
the
False Positive 🦟
Pull Request Test Coverage Report for Build 1499398248
💛 - Coveralls |
|
Closing and opening to run the |
|
No changelog entry? Hadn't added it yet |
I supposed it was voluntary as in "this is a change small enough to not be in the changelog", my bad. |
|
Ah no worries. I think this should go in the changelog though. Could you add an entry? I'm away from computer atm. |
|
Sure, no problem ! |
|
I figured I'd do that in #5397 instead of opening a MR specifically for this. |
| if isinstance(ancestor, nodes.If): | ||
| if ancestor.test.as_string() in TYPING_TYPE_CHECKS_GUARDS: |
There was a problem hiding this comment.
I'm again late, as I'm going through my notifications.
Can't those two be combined? Technically even a return any(...) would be possible, but I think that might be more difficult to read.
There was a problem hiding this comment.
Yeah, should have catched that myself. Going to make a PR with this change!
consider-combining-if seems like a handy checker for our review process 😄
There was a problem hiding this comment.
consider-combining-ifseems like a handy checker for our review process 😄
That is also somewhere on my ideas list 😅
There was a problem hiding this comment.
I found the any to be okay for readability. See #5410
There was a problem hiding this comment.
consider-combining-if seems like a handy checker for our review process
Why didn't I think of that sooner, It's like half the content of my reviews 😄 Also it should trigger on thing like any(bool(e == 1) for e in elements if int(e) == e)). Let me open an issue.
doc/whatsnew/<current release.rst>.Type of Changes
Description
See pylint-dev/astroid#1235 (comment)